Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IF: Create new efficient incremental Merkle tree #2361

Merged
merged 30 commits into from
Apr 2, 2024

Conversation

greg7mdp
Copy link
Contributor

@greg7mdp greg7mdp commented Mar 29, 2024

Resolves #2333

This PR provides a new Merkle tree implementation which does not unnecessarily store extra digests and does not do extra hashes.

Previous versions are renamed: incremental_merkle_tree_legacy, calculate_merkle_legacy.
New versions are named: incremental_merkle_tree, calculate_merkle.

Note: Even though the files have been renamed incremental_merkle_legacy.hpp and merkle_legacy.hpp, the implementation has not been modified (besides remoning the canonical template parameter).

New incremental_merkle_tree

  • does not store extra hashes (root matches calculate_merkle)
  • provides the root on demand, does not compute it on append().
  • is about 20x faster than incremental_merkle_tree_legacy.
  • storage is minimal. Just one uint64_t and a vector<digest_type> whose size() is std::popcount(num_digest_appended).

New calculate_merkle

  • does not store extra hashes. (root matches incremental_merkle_tree).
  • is about 25% faster than calculate_merkle_legacy without multithreading
  • supports internal multithreading for computing roots of collections whose size() > 4096, in which case it is 2x to almost 4x faster (2x for sequences of at least 10k digests, 4x for very large sequences).
  • doesn't mutate the passed deque<digest_type>.

New performance tests

This PR adds two performance tests comparing the legacy and new merkle tree implementations

  • ./unittests/unit_test "--run_test=merkle_tree_tests/perf_test_one_large"
  • ./unittests/unit_test "--run_test=merkle_tree_tests/perf_test_many_small"

New consistency test

This PR adds a test checking consistency between incremental_merkle_tree and calculate_merkle when adding from 1 to 1000 digests:

  • ./unittests/unit_test "--run_test=merkle_tree_tests/consistency_over_large_range"

greg7mdp added 13 commits March 25, 2024 09:57
For 10 levels of recursion, we used ~1540 bytes of stack.

This translates to 5KB of stack space (32 levels of recursion) for appending 4 billion digests
(the max. possible number of blocks).

Since the default Ubuntu stack for a process is 10MB, and 2MB for a thread, this means that, even
for a thread,  we'd use at most 5/2000, or 0.25% of the stack available for any append().

If the stack is already 99.75% used before `incremental_merkle_tree::append()` is called, we have a
problem elsewhere.
@greg7mdp greg7mdp linked an issue Mar 29, 2024 that may be closed by this pull request
@greg7mdp greg7mdp requested review from heifner and linh2931 March 29, 2024 20:50
libraries/chain/controller.cpp Outdated Show resolved Hide resolved
return hash_combine(start[0], start[1]);
else {
if (async && size >= 4096) { // below 4096, starting async threads is overkill
std::array<std::future<digest_type>, 4> fut; // size dictates the number of threads (must be power of two)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is likely fine as it is only 4 threads. We do need to be mindful of thread usage as we don't want to overwhelm a system so that it slows down the main thread. Other thread usage is setup at startup. Would be nice to be able to provide an io_context for these tasks so they could run on the chain thread pool; however likely not worth the effort.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated it so that it uses only 2 threads up to 4096 digests, and 4 for 4096 and up.

libraries/chain/include/eosio/chain/merkle_legacy.hpp Outdated Show resolved Hide resolved
libraries/chain/include/eosio/chain/merkle_legacy.hpp Outdated Show resolved Hide resolved
libraries/chain/include/eosio/chain/merkle_legacy.hpp Outdated Show resolved Hide resolved
libraries/chain/include/eosio/chain/merkle.hpp Outdated Show resolved Hide resolved
@ericpassmore
Copy link
Contributor

Note:start
group: IF
category: INTERNALS
summary: Improved Merkle Tree, no longer storing unneeded digests, switched to incremental calculations.
Note:end

@greg7mdp greg7mdp merged commit 8c03a62 into hotstuff_integration Apr 2, 2024
34 checks passed
@greg7mdp greg7mdp deleted the gh_2333 branch April 2, 2024 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IF: Create new efficient incremental Merkle tree
4 participants